-
Notifications
You must be signed in to change notification settings - Fork 799
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add limit for store gateway downloaded bytes #5179
Conversation
He @yeya24 do we really need this in addition to |
It is kind of tricky. How Store Gateway works is that based on the matchers of user query, it will try to fetch postings either from cache or from S3. I think this limit can cover some edge cases but I am also not sure if it is good to have that. As we don't have the visibility for that and it is enforced per gRPC request, not per query. |
I think this limit is still useful to protect store-gateways from getting OOM killed. We enforce the |
cc17ef3
to
ec8caaf
Compare
LGTM! In the future we may consider deprecating the other ones? Did it help to prevent OOM? |
What are the other ones you mentioned here? The series and chunk limit on store gateway? |
Signed-off-by: Ben Ye <benye@amazon.com>
Signed-off-by: Ben Ye <benye@amazon.com>
Signed-off-by: Ben Ye <benye@amazon.com>
Signed-off-by: Ben Ye <benye@amazon.com>
ee97953
to
57f11f6
Compare
Merging it now. |
What this PR does:
Add the limit from thanos-io/thanos#5801 to Cortex. This is a store gateway only limit that limits all data downloaded for each gRPC request to store gateway.
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]